Walkthrough

Removes the redoc-express dependency and /api/docs Redoc UI route, replacing initSwagger with initApiSpec that serves the merged OpenAPI spec at GET /api/spec.json. Simultaneously adds unauthenticated GET /api/public/docs and GET /api/public/docs/:slug.md endpoints backed by a TTL-cached service that parses on-disk markdown guides into a structured category tree driven by config.docs.guideSections.

Changes

Redoc Decommission → initApiSpec

Public Docs Content Contract

Sequence Diagram

sequenceDiagram
  participant Client
  participant PublicDocsRoute as GET /api/public/docs
  participant PublicDocsController
  participant PublicDocsService
  participant DocsTreeHelper as publicDocs.tree.js
  participant Filesystem as doc/guides/*.md

  Client->>PublicDocsRoute: GET /api/public/docs (unauthenticated)
  PublicDocsRoute->>PublicDocsController: tree(req, res)
  PublicDocsController->>PublicDocsService: getTree()
  PublicDocsService->>PublicDocsService: check TTL cache
  alt cache miss
    PublicDocsService->>DocsTreeHelper: loadGuideEntries(guideFiles)
    DocsTreeHelper->>Filesystem: readFileSync each .md
    Filesystem-->>DocsTreeHelper: markdown content
    DocsTreeHelper-->>PublicDocsService: sorted guide entries
    PublicDocsService->>DocsTreeHelper: buildDocsTree(entries, guideSections)
    DocsTreeHelper-->>PublicDocsService: { categories }
    PublicDocsService->>PublicDocsService: stamp expiresAt, build slug index
  end
  PublicDocsService-->>PublicDocsController: { categories }
  PublicDocsController->>Client: 200 + Cache-Control + success envelope

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• pierreb-devkit/Node#3453: Both PRs modify lib/services/express.js and lib/helpers/guides.js to merge markdown guides into spec.info.description; this PR renames the entrypoint to initApiSpec while PR #3453 introduced the guide-loading/merging logic.
• pierreb-devkit/Node#3460: Both PRs affect how markdown guides are loaded and merged into /api/spec.json's info.description; PR #3460 altered the guide-loader tests while this PR changes the wiring entry point to initApiSpec.
• pierreb-devkit/Node#3318: This PR's removal of /api/docs from modules/audit/middlewares/audit.middleware.js SKIP_PREFIXES directly builds on the audit middleware introduced in PR #3318.

Suggested labels

Feat

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.67%. Comparing base (3937953) to head (4c1636e).

@@            Coverage Diff             @@
##           master    #3878      +/-   ##
==========================================
+ Coverage   92.50%   92.67%   +0.17%     
==========================================
  Files         165      169       +4     
  Lines        5403     5529     +126     
  Branches     1740     1775      +35     
==========================================
+ Hits         4998     5124     +126     
  Misses        325      325              
  Partials       80       80              

Addressed review findings (Copilot + CodeRabbit):

• Naming: renamed publicDocs.* → public.docs.* (devkit {module}.{entity}.{type} convention, plural *.routes.js), via git mv (history preserved).
• Defensive copies: guideFiles()/guideSections() now return copies ([...] / .map(s=>({...s}))) — no shared mutable config state. (Was already fixed in c64335e; the earlier CodeRabbit note referenced a pre-rename revision.)
• Deterministic ordering: glob list sorted before fallback indexing + sort tiebreaker on equal prefixes.
• /api/docs decom test: added a positive assertion that it falls through to the generic catch-all landing (200), so the decommission contract is unambiguous.
• Cache-Control: public, max-age=300 on the docs responses (not on 404).
• JSDoc on async controller handlers (@returns {Promise<void>}) + test helpers; integration beforeAll no longer masks bootstrap errors.
  CI green; coverage held (new module 100%).

@coderabbitai review — all prior findings addressed (naming convention, defensive copies, deterministic ordering, Cache-Control, the /api/docs decom assertion, JSDoc). Please re-review.

#!/bin/bash
set -euo pipefail

echo "=== Listing modules/public structure ==="
find modules/public -type f | sort

echo ""
echo "=== modules/public/controllers ==="
find modules/public/controllers -type f | sort

echo ""
echo "=== modules/public/services ==="
find modules/public/services -type f | sort

echo ""
echo "=== modules/public/helpers ==="
find modules/public/helpers -type f | sort

echo ""
echo "=== modules/public/routes ==="
find modules/public/routes -type f | sort

echo ""
echo "=== modules/public/tests ==="
find modules/public/tests -type f | sort

#!/bin/bash
set -euo pipefail

echo "=== Controller file ==="
cat modules/public/controllers/public.docs.controller.js 2>/dev/null || find modules/public/controllers -name "*.js" -exec echo "File: {}" \; -exec cat {} \;

echo ""
echo "=== Service file ==="
find modules/public/services -name "*.js" -exec echo "File: {}" \; -exec cat {} \;

echo ""
echo "=== Helper/tree file ==="
find modules/public/helpers -name "*.js" -exec echo "File: {}" \; -exec cat {} \;

echo ""
echo "=== Routes file ==="
find modules/public/routes -name "*.js" -exec echo "File: {}" \; -exec cat {} \;

#!/bin/bash
set -euo pipefail

echo "=== Test files ==="
find modules/public/tests -name "*.js" -exec echo "File: {}" \; -exec cat {} \;

#!/bin/bash
set -euo pipefail

echo "=== lib/services/express.js ==="
cat lib/services/express.js

echo ""
echo "=== lib/helpers/guides.js ==="
cat lib/helpers/guides.js

echo ""
echo "=== config/defaults/development.config.js ==="
cat config/defaults/development.config.js

#!/bin/bash
set -euo pipefail

echo "=== modules/core/tests/core.integration.tests.js (relevant section) ==="
grep -n "api/docs\|spec.json\|Redoc\|OpenAPI\|decommission\|catch-all\|landing" modules/core/tests/core.integration.tests.js -A 5 -B 2 || true

echo ""
echo "=== modules/audit/middlewares/audit.middleware.js ==="
cat modules/audit/middlewares/audit.middleware.js

echo ""
echo "=== modules/home/doc/guides/ ==="
ls modules/home/doc/guides/ 2>/dev/null || echo "not found"
find . -path "*/home/doc/guides/*" -type f 2>/dev/null | sort

echo ""
echo "=== package.json (dependencies section) ==="
python3 -c "
import json
with open('package.json') as f:
    pkg = json.load(f)
deps = pkg.get('dependencies', {})
for k in sorted(deps.keys()):
    print(f'{k}: {deps[k]}')
" 2>/dev/null || grep -A 3 '"redoc\|posthog\|resend' package.json || true

@PierreBrisorgueil: Thorough re-review completed against the current branch. All previously flagged findings are confirmed addressed.

Review Summary

Naming convention ✅
All files follow {module}.{entity}.{type} — public.docs.controller.js, public.docs.service.js, public.docs.tree.js, public.docs.routes.js (plural) — and tests mirror that pattern.

Defensive copies ✅
guideFiles() returns [...config.files.guides] and guideSections() returns .map((s) => ({ ...s })). Neither leaks mutable config state.

Deterministic ordering ✅
loadGuideEntries pre-sorts the raw file paths with [...filePaths].sort() before assigning fallback indices, then applies a stable final sort with a.order - b.order || a.slug.localeCompare(b.slug) as the tiebreaker. Glob non-determinism is fully neutralised.

/api/docs decom test ✅
The integration test makes a positive assertion — expect(res.text).toContain('Devkit Node Api') — confirming the 200 is the generic catch-all, not a surviving Redoc surface.

Cache-Control ✅
public, max-age=300 is set before the 200 on both tree and raw; the 404 path returns without touching that header.

JSDoc ✅
Both tree and raw carry @returns {Promise<void>}. mockResponse in the controller unit test has a typed @returns as well.

Additional checks that passed:

• DEFAULT_PERSONA is Object.freeze(['all']); sectionForEntry always returns a copy ([...match.persona] / [...DEFAULT_PERSONA]).
• compute() is fully synchronous (fs.readFileSync); the comment explaining why no inflight guard is needed is present and correct.
• redoc-express is absent from package.json; /api/docs removed from SKIP_PREFIXES.
• Fallback integration: config.docs.guideSections prefixMin: 0, prefixMax: 9 covers both shipped guides (00-welcome, 01-quickstart), and the integration test asserts the real get-started grouping and ['all'] persona.
• Every test file stubs loadGuides inline (no module-level mocks).

One minor note (no action required): buildDocsTree assigns categories in first-guide discovery order rather than sections array order. With the current design — entries sorted by numeric prefix, sections using non-overlapping prefix ranges — output category order naturally tracks config section order. Just worth documenting if the schema ever allows overlapping ranges.

The implementation is clean, well-structured, and the contract is unambiguous. Happy to approve.

[approve]